Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sai's solar system #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Sai's solar system #32

wants to merge 2 commits into from

Conversation

ssamant
Copy link

@ssamant ssamant commented Feb 15, 2017

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
What was the purpose of the initialize method in your class? set the instance variables I would be using within the class. initialize any values that would be passed from the parameters (e.g. formation
Describe an instance variable you used and what you used it for. I used the @all_planets variable to access my array of planets. I could call Planet class methods on elements within @all_planets and I used those within methods in SolarSystem class
Describe what the difference would be if your SolarSystem used an Array vs a Hash.
Do you feel like you used consistent indentation throughout your code? Yes. However, I used the auto-indent shortcut to double check but it went crazy and super-indented everything so I had to re-edit.

@CheezItMan
Copy link

Solar System

What We're Looking For

Feature Feedback
Created Custom Class with initialize method & instance variables. Yes, nice work using the hash to set the planet instance variables.
Used an Array to store a list of planets in the SolarSystem class. Done
Readable code with consistent indentation. Done
Created a pull request with your name & a meaningful message. Yes, see below about the hash solar system question.
Extras
Calculating distance between two planets Nicely Done
User interaction Nicely Done

Summary

For the question about using a Hash in SolarSystem, using a hash would require a key to directly access individual planets. To iterate through them it would also require getting the list of keys and iterating through the keys.

Mixing Star Wars & Star Trek references, funny.

Overall a very nice job, good work on the extras.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants